Skip to content

test: fix race in test-net-server-pause-on-connect#4637

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:fix-net-server-race
Closed

test: fix race in test-net-server-pause-on-connect#4637
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:fix-net-server-race

Conversation

@Trott

@Trott Trott commented Jan 12, 2016

Copy link
Copy Markdown
Member

A 50ms timeout results in a race condition. Instead, enforce expected
order through callbacks. This has the side effect of speeding up the
test in most situations.

Ref: #4476

R=@jbergstroem
R=@brendanashworth

@Trott Trott added the test Issues and PRs related to the tests. label Jan 12, 2016
@jbergstroem

Copy link
Copy Markdown
Member

@mscdex mscdex added the net Issues and PRs related to the net subsystem. label Jan 12, 2016
@jbergstroem

Copy link
Copy Markdown
Member

LGTM

@jasnell

jasnell commented Jan 12, 2016

Copy link
Copy Markdown
Member

LGTM but there was on failure in CI. Can you take a second to confirm that it's unrelated?

@Trott

Trott commented Jan 13, 2016

Copy link
Copy Markdown
Member Author

#4615 is supposed to address some or all of those unrelated failures, so maybe re-run a CI of this rebased after that lands.

A 50ms timeout results in a race condition. Instead, enforce expected
order through callbacks. This has the side effect of speeding up the
test in most situations.

Ref: nodejs#4476
@Trott Trott force-pushed the fix-net-server-race branch from 622e2e9 to 70f85b4 Compare January 13, 2016 03:51
@Trott

Trott commented Jan 13, 2016

Copy link
Copy Markdown
Member Author

#4615 landed, this branch/PR rebased, and now running CI again:

https://ci.nodejs.org/job/node-test-commit/1711/

@jasnell

jasnell commented Jan 13, 2016

Copy link
Copy Markdown
Member

Looks good. landing...

jasnell pushed a commit that referenced this pull request Jan 13, 2016
A 50ms timeout results in a race condition. Instead, enforce expected
order through callbacks. This has the side effect of speeding up the
test in most situations.

Ref: #4476
PR-URL: #4637
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell

jasnell commented Jan 13, 2016

Copy link
Copy Markdown
Member

Landed in e22cc6c

@jasnell jasnell closed this Jan 13, 2016
rvagg pushed a commit that referenced this pull request Jan 14, 2016
A 50ms timeout results in a race condition. Instead, enforce expected
order through callbacks. This has the side effect of speeding up the
test in most situations.

Ref: #4476
PR-URL: #4637
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 28, 2016
A 50ms timeout results in a race condition. Instead, enforce expected
order through callbacks. This has the side effect of speeding up the
test in most situations.

Ref: #4476
PR-URL: #4637
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
A 50ms timeout results in a race condition. Instead, enforce expected
order through callbacks. This has the side effect of speeding up the
test in most situations.

Ref: #4476
PR-URL: #4637
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 11, 2016
A 50ms timeout results in a race condition. Instead, enforce expected
order through callbacks. This has the side effect of speeding up the
test in most situations.

Ref: nodejs#4476
PR-URL: nodejs#4637
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 11, 2016
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 15, 2016
A 50ms timeout results in a race condition. Instead, enforce expected
order through callbacks. This has the side effect of speeding up the
test in most situations.

Ref: nodejs#4476
PR-URL: nodejs#4637
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
A 50ms timeout results in a race condition. Instead, enforce expected
order through callbacks. This has the side effect of speeding up the
test in most situations.

Ref: nodejs#4476
PR-URL: nodejs#4637
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott Trott deleted the fix-net-server-race branch January 13, 2022 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants